-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat(android,ios): add request headers support #363
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
Looking for the same thing, will this become a feature in the short term? |
I would like this feature as well. Any ETAs on this? |
You can help by testing this locally (via the fork of @al1lhomme that contains these changes) and reporting back if it worked as expected. |
Could I ask for progress? This feature is all I need to download any type of files from my server with authentication Header. |
@janpio Just merged this into our fork and it seems to be working. It sends headers on the first request and on page reload. Tested on iOS and Android. Did not test using IAB's WKWebView mode. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added some comments.
Could you also rebase your change?
var strWindowHeaders = ''; | ||
if (windowHeaders) { | ||
if (typeof windowHeaders === 'string' || windowHeaders instanceof String) { | ||
strWindowHeaders = windowHeaders.replace(/@/gi, '@a'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what exactly is the reason for all the replacements of @
and =
if you just re-replace it in the corresponding Java/Objective-C code?
Wouldn't it be more readable and clearer to pass in a JSON object and then use Android's included JSONObject support to create a HashMap and iOS' included NSJSONSerialization?
This seems the same as PR #115 - but 3 years newer with fewer conflicts. |
If this feature will allow us to override the User-Agent header, it would fulfill our feature request #502 |
Resolved the conflict. |
@@ -271,7 +272,7 @@ public void run() { | |||
} else { | |||
((InAppBrowserClient)inAppWebView.getWebViewClient()).waitForBeforeload = false; | |||
} | |||
inAppWebView.loadUrl(url); | |||
inAppWebView.loadUrl(url, null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why all those changes to the loadUrl
calls if we don't actually change the method signature ourselves? Couldn't we just keep these as they are and only add the headers
in the places where we need it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this was to explicitely mark headers were null for easier code navigation
Have to test this for both platforms, and on iOS for both webview types. Someone have a page handy that shows the headers that were used to call it? Then it should be pretty simple to create a test app. We should probably also try to add this to the tests somehow. |
|
Any update on this? |
We will have to find solutions to the comments that are open here. Some questions regarding the code, no new tests, no tested yet. Anyone who wants to jump in and is not a contributor here (who can edit the fork directly) can just fork at https://github.com/al1lhomme/cordova-plugin-inappbrowser and then create a new PR to this repo and let us know here via a comment. |
how can i use the version with custom headers? |
Hi, I have tested the PullRequest in Android and it works ok. Surprisingly, adding the new header "Google-Accounts-Check-OAuth-Login:true", Google Sign seems to be working. I am using browserTarget: '_blank' |
I have tested and used the PullRequest in Android and iOS. Headers are working. |
Doesn't work for me. Tested with Cordova Android 9.0.0. Made all changes in InAppBrowser.java, inappbrowser.js and index.js |
Any news about this PR? |
Can confirm this works on Android 10, we did not try to patch iOS because our ionic wrapper uses SafariWebviewController and not InAppBrowser thx @al1lhomme |
Helllo guys, any news on this PR? It's been pending since 2018... Being able to pass headers is quite important and useless for several cases... |
Platforms affected
android, ios
What does this PR do?
add request headers support
What testing has been done on this change?
automated test still working
test using headers in my app
closes #361
replaces #115